Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve filter exclusion setting for shade plugin #17824

Merged
merged 1 commit into from
Aug 14, 2023

Conversation

apc999
Copy link
Contributor

@apc999 apc999 commented Jul 23, 2023

What changes are proposed in this pull request?

  1. Improve filter setting in dora/shaded/client/pom.xml to eliminate some warnings in the shading process.
  2. Remove duplicated code of filter setting in UFS. The shading filter in each UFS will inherit the shading plugin settings from dora/underfs/pom.xml, enabling us to reduce duplication by moving the common settings to dora/underfs/pom.xml.

Why are the changes needed?

Code hygiene and readability

Does this PR introduce any user facing changes?

No

@apc999 apc999 changed the title Improve pom for shade plugin Improve filter exclusion setting for shade plugin Jul 23, 2023
@apc999 apc999 requested a review from Xenorith July 24, 2023 17:50
@apc999 apc999 added the type-code-quality code quality improvement label Jul 24, 2023
Copy link
Contributor

@Xenorith Xenorith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i checked the other ufs module pom.xml. for the ones with a dependency on alluxio-underfs-hdfs, their shade plugin configuration usually contains this filter entry:

                <filter>
                  <artifact>org.alluxio:alluxio-underfs-hdfs</artifact>
                  <excludes>
                    <!-- Because this module depends on alluxio-underfs-hdfs, eliminate HDFS factory implementation -->
                    <exclude>alluxio/underfs/hdfs/HdfsUnderFileSystemFactory.*</exclude>
                    <exclude>META-INF/services/alluxio.underfs.UnderFileSystemFactory</exclude>
                  </excludes>
                </filter>

the following diff makes them all consistent:

diff --git a/dora/underfs/cephfs-hadoop/pom.xml b/dora/underfs/cephfs-hadoop/pom.xml
index 89d409a60c..fccb21bee9 100644
--- a/dora/underfs/cephfs-hadoop/pom.xml
+++ b/dora/underfs/cephfs-hadoop/pom.xml
@@ -64,6 +64,7 @@
                   <artifact>org.alluxio:alluxio-underfs-hdfs</artifact>
                   <excludes>
                     <!-- Because this module depends on alluxio-underfs-hdfs, eliminate HDFS factory implementation -->
+                    <exclude>alluxio/underfs/hdfs/HdfsUnderFileSystemFactory.*</exclude>
                     <exclude>META-INF/services/alluxio.underfs.UnderFileSystemFactory</exclude>
                   </excludes>
                 </filter>
diff --git a/dora/underfs/ozone/pom.xml b/dora/underfs/ozone/pom.xml
index 8babf6aae2..9b9411c53a 100644
--- a/dora/underfs/ozone/pom.xml
+++ b/dora/underfs/ozone/pom.xml
@@ -147,6 +147,8 @@
                 <filter>
                   <artifact>org.alluxio:alluxio-underfs-hdfs</artifact>
                   <excludes>
+                    <!-- Because this module depends on alluxio-underfs-hdfs, eliminate HDFS factory implementation -->
+                    <exclude>alluxio/underfs/hdfs/HdfsUnderFileSystemFactory.*</exclude>
                     <exclude>META-INF/services/alluxio.underfs.UnderFileSystemFactory</exclude>
                   </excludes>
                 </filter>

@apc999
Copy link
Contributor Author

apc999 commented Aug 14, 2023

@Xenorith comment addressed. PTAL

@apc999
Copy link
Contributor Author

apc999 commented Aug 14, 2023

alluxio-bot, merge this please

@alluxio-bot alluxio-bot merged commit 3d8fbda into Alluxio:main Aug 14, 2023
11 checks passed
@apc999 apc999 deleted the pomfix branch August 14, 2023 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-code-quality code quality improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants